Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.04%. Comparing base (d4980dd) to head (5fafb8f).
Report is 48 commits behind head on develop.

@@             Coverage Diff             @@
##           develop    #6274      +/-   ##
===========================================
+ Coverage    81.96%   82.04%   +0.08%     
===========================================
  Files          543      543              
  Lines       345677   345677              
  Branches       323      323              
===========================================
+ Hits        283332   283626     +294     
+ Misses       62337    62043     -294     
  Partials         8        8              

The changes seem fine, but the question to ask is - should we be running this job on every PR/commit? it's an expensive job to run indicated by the timing (22m of runtime), and that was part of the reason we removed the docker image build on each PR/commit (deferring to run it manually when needed).

leaving it as-is isn't a bad thing either - i just see it as a tradeoff: we may find that PR's take longer to merge, and will have to possibly ignore failures when this job takes longer than usual (the other reason we stopped running the docker image build in PR's).

The changes seem fine, but the question to ask is - should we be running this job on every PR/commit? it's an expensive job to run indicated by the timing (22m of runtime), and that was part of the reason we removed the docker image build on each PR/commit (deferring to run it manually when needed).

leaving it as-is isn't a bad thing either - i just see it as a tradeoff: we may find that PR's take longer to merge, and will have to possibly ignore failures when this job takes longer than usual (the other reason we stopped running the docker image build in PR's).

Fair points! I have just re-run the CI after merging develop, let's see if the runtime improves since it should cache some of the outputs. If that doesn't, I think it is OK to treat this the same as Docker builds.

Unfortunately it's still in the 20 minutes range. I am not sure if we want to explore it, (I could try it out myself when I merge my current work), but there could be a chance to maybe improve this by a lot with https://github.com/cachix/cachix-action . They offer a 5GB free cache for open source projects that I believe should be enough for this

It seems to be spending time unpacking from the cache, I think, so I am not sure we would get an improvement (but it is worth trying). Is 20m really a problem? The other CI tests take 1h (at least on this PR).

If we believe it's a problem, I'll just replicate the Docker build CI workflow here.

I switched to nix flake check and configured it so that it does cargo check under the hood. This run completed in less than 2m from a warm cache, while the previous run took 20m to build the cache (mostly the dependencies).

I am not 100% sure when/how cache will need to be re-evaluated, but I think that 1. while keeping the flake unchanged and 2. keeping the dependencies unchanged we should be able to re-use that and significantly reduce evaluation time.

I switched to nix flake check and configured it so that it does cargo check under the hood. This run completed in less than 2m from a warm cache, while the previous run took 20m to build the cache (mostly the dependencies).

I am not 100% sure when/how cache will need to be re-evaluated, but I think that 1. while keeping the flake unchanged and 2. keeping the dependencies unchanged we should be able to re-use that and significantly reduce evaluation time.

nice! one nit: https://github.com/stacks-network/stacks-core/actions/runs/16260400949/job/45906555094#step:4:34

is it possible to skip this step in CI? not a blocker, just noticed the annotation

I switched to nix flake check and configured it so that it does cargo check under the hood. This run completed in less than 2m from a warm cache, while the previous run took 20m to build the cache (mostly the dependencies).
I am not 100% sure when/how cache will need to be re-evaluated, but I think that 1. while keeping the flake unchanged and 2. keeping the dependencies unchanged we should be able to re-use that and significantly reduce evaluation time.

nice! one nit: https://github.com/stacks-network/stacks-core/actions/runs/16260400949/job/45906555094#step:4:34

is it possible to skip this step in CI? not a blocker, just noticed the annotation

I have just tried pushing c4e81fa which explicitly disables flakehub, let's see if that gets the job done.

I switched to nix flake check and configured it so that it does cargo check under the hood. This run completed in less than 2m from a warm cache, while the previous run took 20m to build the cache (mostly the dependencies).
I am not 100% sure when/how cache will need to be re-evaluated, but I think that 1. while keeping the flake unchanged and 2. keeping the dependencies unchanged we should be able to re-use that and significantly reduce evaluation time.

nice! one nit: https://github.com/stacks-network/stacks-core/actions/runs/16260400949/job/45906555094#step:4:34
is it possible to skip this step in CI? not a blocker, just noticed the annotation

I have just tried pushing c4e81fa which explicitly disables flakehub, let's see if that gets the job done.

looks good!